fix(acp): isolate Claude sessions from host credentials - #4611
Open
jmecom wants to merge 5 commits into
Open
Conversation
Co-authored-by: Jordan Mecom <jm@squareup.com> Signed-off-by: Jordan Mecom <jm@squareup.com>
Co-authored-by: Jordan Mecom <jm@squareup.com> Signed-off-by: Jordan Mecom <jm@squareup.com>
Recognize Claude adapters launched through a JS runner (`npx`, `node`, `bunx`) by the package in their arguments. Matching the command name alone spawned the same adapter with no isolation, and the fallback was silent; a Claude-looking command that still cannot be confirmed now warns that it is unisolated, and a confirmed one logs that it is. Project `user.name`, `user.email`, and a `nostr` credential helper from the operator's global git config into the disposable profile. Redirecting HOME hides ~/.gitconfig, which otherwise costs the agent its commit identity and its NIP-98 push credential against Buzz's own git server. Only those keys cross the boundary: a keychain or `!shell` helper would hand the agent the operator's stored credentials, and `GIT_CONFIG_GLOBAL` now passes through as the explicit opt-out. Also allowlist the proxy variables — an egress-proxied host has no route to the model API without them, and they carry no credential material — plus `USERNAME`, and redirect `HOMEDRIVE`/`HOMEPATH` so Windows tooling that predates `USERPROFILE` cannot resolve the operator's home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Eli Foster <efoster@squareup.com>
The pure tests only assert the JSON the harness emits, so a wrong `_meta` key or an over-tight allowlist would pass them. This ignored test drives the real claude-agent-acp through the isolated spawn path and requires `session/new` to be accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Eli Foster <efoster@squareup.com>
A disposable HOME gives hermit, rustup, npm, and pub a cold cache, so every Claude session re-downloaded its whole toolchain: 13.4s and a 278MB hermit/node fetch for a session that does nothing. The caches now point at a Buzz-owned directory outside the profile, which drops a warm session to 0.6s. The directory is Buzz's own rather than the operator's cache: agents write to it, and a poisoned toolchain must not reach the operator's builds. CARGO_HOME stays unset because it holds credentials.toml and cargo has no cache-only knob, so a shared registry would also share registry tokens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Eli Foster <efoster@squareup.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude adapters were spawned with the harness's environment, so an agent ran as the operator: user-scope MCP servers from
~/.claude.json, the claude.ai connectors that identity has authorized, and the Claude credentials in the keychain. An agent reading untrusted channel messages had both a read and an exfiltration path by default.The adapter now starts with
env_clear()plus a small allowlist (PATH, locale, terminal, proxy, CA bundle, Windows system paths) and a disposable temp profile:HOME,USERPROFILE,XDG_{CONFIG,CACHE,DATA}_HOME,CLAUDE_CONFIG_DIR,APPDATA/LOCALAPPDATA, andHOMEDRIVE/HOMEPATHon Windows.settingSources: []is also sent onsession/new, but the profile redirect is what removes the operator's config —settingSourcesgovernssettings.jsononly, not~/.claude.jsonor account identity.This is not an OS sandbox: it removes automatic inheritance, not deliberate absolute-path access by an otherwise-authorized tool.
Four consequences worth agreeing on before merge:
ANTHROPIC_API_KEYsurface, so the path exists; existing claude-code personas stop working until it is filled in.HOMEalone gives hermit a cold state dir, so every session re-downloads its toolchain — measured at 13.4s and a 278MB hermit/node fetch for a session that does nothing.HERMIT_STATE_DIR,RUSTUP_HOME,npm_config_cache, andPUB_CACHEtherefore point at<host cache>/buzz/agent-toolchains, outside the profile; a warm session is 0.6s. That directory is Buzz's own rather than the operator's cache, so an agent that poisons a toolchain cannot reach the operator's builds — but it is shared between agent sessions.CARGO_HOMEstays unset: it holdscredentials.tomland cargo has no cache-only knob, so sharing the registry would share registry tokens. Rust sessions pay a registry download as a result.HOMEhides~/.gitconfig, costing the agent its commit identity and its NIP-98 push credential. The profile therefore gets a.gitconfigwithuser.name,user.email, and acredential.helperonly when it resolves togit-credential-nostr; keychain,store, and!shellhelpers are dropped, since handing those over is the access this change removes.GIT_CONFIG_GLOBALpasses through as a one-variable opt-out.npx/node/bunxwith aclaude-agent-acp/claude-code-acpargument); a command that looks like Claude but cannot be confirmed logs a warning that it is not isolated rather than failing open silently.Testing
env -u BUZZ_ACP_LAZY_POOL bin/cargo test -p buzz-acp: 683 lib + 9 integration tests passcargo clippy -p buzz-acp --all-targets -- -D warnings,cargo fmt -- --check: cleanUnit tests cover the allowlist, indirect adapter detection, the git-config projection and its rejections, the Windows home split, and the shared-cache path per platform; spawn tests assert what the child actually observes, including that a persona cannot point
HOMEback at the operator and that the hermit state dir lands outside the disposable profile whileCARGO_HOMEstays unset. That last assertion was checked against a neutered implementation.Verified against the real adapter (
@agentclientprotocol/claude-agent-acp0.64.2):claude mcp listinitializeauthMethods: []— already the operatorclaude-ai-login,console-loginclaude -pWith a bogus
ANTHROPIC_API_KEYin the isolated profile the error becomes "Invalid API key", confirming a persona-supplied credential takes effect.isolated_claude_adapter_starts_and_accepts_the_session_metadata(#[ignore], needs the adapter on PATH) drives the real binary through this spawn path and gets a session back — cold 13.4s, warm 0.6s. Caveat: that shows_meta.claudeCode.options.settingSourcesis accepted, not that it is honored.Originating Buzz thread:
buzz://message?channel=3928fe05-df61-4b5d-b9c7-d623b9b10ea1&id=3c6c02312f763fbe0d2bfc33a6c1a362f91d0354f3d18b039cf7a0558c1439d1